Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tests-Only] Add acceptance tests for enforced expiration on collaborators share #3159

Merged
merged 1 commit into from
Mar 17, 2020

Conversation

kiranparajuli589
Copy link
Contributor

@kiranparajuli589 kiranparajuli589 commented Mar 6, 2020

Description

Acceptance test added for enforced expiration date on a user/group share

Related Issue

Motivation and Context

How Has This Been Tested?

  • locally

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • ...

@kiranparajuli589 kiranparajuli589 self-assigned this Mar 6, 2020
@kiranparajuli589 kiranparajuli589 requested review from haribhandari07, dpakach and jasson99 and removed request for haribhandari07 March 6, 2020 11:09
@kiranparajuli589 kiranparajuli589 force-pushed the change-enforced-expiration branch from 1217f90 to 4430f33 Compare March 10, 2020 03:37
@kiranparajuli589 kiranparajuli589 force-pushed the change-enforced-expiration branch 7 times, most recently from 8468f05 to f906430 Compare March 12, 2020 08:07
@kiranparajuli589 kiranparajuli589 marked this pull request as ready for review March 12, 2020 08:10
Copy link
Contributor

@jasson99 jasson99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kiranparajuli589 kiranparajuli589 force-pushed the change-enforced-expiration branch 2 times, most recently from c955ea7 to 351d6de Compare March 13, 2020 06:18
@kiranparajuli589 kiranparajuli589 force-pushed the change-enforced-expiration branch from 351d6de to 46301da Compare March 13, 2020 06:30
Copy link
Contributor

@dpakach dpakach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

tests/acceptance/stepDefinitions/sharingContext.js Outdated Show resolved Hide resolved
tests/acceptance/stepDefinitions/sharingContext.js Outdated Show resolved Hide resolved
@kiranparajuli589 kiranparajuli589 force-pushed the change-enforced-expiration branch 2 times, most recently from 9a62d92 to 0e02685 Compare March 13, 2020 10:21
Copy link
Contributor

@haribhandari07 haribhandari07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

And the setting "shareapi_enforce_expire_date_user_share" of app "core" has been set to "yes"
And the setting "shareapi_expire_after_n_days_user_share" of app "core" has been set to "5"
And user "user1" has logged in using the webUI
When the user tries to share file "<shared-resource>" with user "User Two" which expires in "+6" days using the webUI
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When the user tries to share file "<shared-resource>" with user "User Two" which expires in "+6" days using the webUI
When the user tries to share the resource "<shared-resource>" with user "User Two" which expires in "+6" days using the webUI

should not say file, because one of the examples is a folder

| simple-folder |

@issue-3174
Scenario Outline: user cannot share with user with expiration date within set enforced maximum date if default expiry date for both user and group is enabled
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Scenario Outline: user cannot share with user with expiration date within set enforced maximum date if default expiry date for both user and group is enabled
Scenario Outline: enforced expiry date for groups does not affect user shares

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need also a test for the other direction

const { client } = require('nightwatch-api')
const collaboratorDialog = client.page.FilesPageElement.SharingDialog.collaboratorsDialog()
const SHARE_TYPE_STRING = {
user: 'user',
group: 'group',
federation: 'remote'
}
const COLLABORATOR_PERMISSION_ARRAY = sharingHelper.COLLABORATOR_PERMISSION_ARRAY
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unneeded

@@ -1,13 +1,14 @@
const util = require('util')
const _ = require('lodash')
const { COLLABORATOR_PERMISSION_ARRAY } = require('../../helpers/sharingHelper')
const sharingHelper = require('../../helpers/sharingHelper')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unneeded

.expirationDatePicker()
.setExpirationDate(days)
const dateToSet = sharingHelper.calculateDate(days)
const expectToSucceed = await this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

variable name does not make sense, how does setExpirationDate know what is expected?

@@ -1009,3 +1016,28 @@ Then('the fields of the {string} collaborator for file/folder/resource {string}
Then('user {string} should have received a share with target {string} and expiration date in {int} day/days', function (user, target, days) {
return checkReceivedSharesExpirationDate(user, target, days)
})

Then('the expiration date input field of collaborators form should have value with {string} days on the webUI',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Then('the expiration date input field of collaborators form should have value with {string} days on the webUI',
Then('the expiration date shown on the webUI should be {string} days',

@haribhandari07 haribhandari07 force-pushed the change-enforced-expiration branch from 0e02685 to a713423 Compare March 16, 2020 07:05
Comment on lines 701 to 702
And the setting "shareapi_expire_after_n_days_user_share" of app "core" has been set to "10"
And the setting "shareapi_expire_after_n_days_group_share" of app "core" has been set to "5"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be the other way around

@haribhandari07 haribhandari07 force-pushed the change-enforced-expiration branch 2 times, most recently from 6025297 to 82c2ed9 Compare March 16, 2020 10:18
@haribhandari07 haribhandari07 force-pushed the change-enforced-expiration branch from 82c2ed9 to c723f81 Compare March 16, 2020 11:09
Copy link
Contributor

@dpakach dpakach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@individual-it individual-it merged commit 2e2c93b into master Mar 17, 2020
@delete-merged-branch delete-merged-branch bot deleted the change-enforced-expiration branch March 17, 2020 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status:Needs-Review Needs review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants